Support passing argc and argv to target program #185

ChinYikMing · 2023-08-06T22:51:55Z

Half close #150, I left envp in TODO

I assumed all emulator's option are passed before the target program and its args thus I modify the parse_args function consequently

src/main.c

Makefile

src/main.c

jserv

Resolve the concerns raised by CodeQL.

src/main.c

src/riscv.c

tests/args.c

- half close #150, I left envp in TODO - refactor parse_args function with getopt to support short option which looks cleaner and replace all long option with short option - update README

src/riscv.c

qwe661234 · 2023-08-08T02:27:31Z

In file src/risc.c, the memory_write has been invoked twice to wrtie argc or args to memory (according to your comments). Why we need to maintain two copies of argc or args in virtual memory?

ChinYikMing · 2023-08-08T02:35:14Z

In file src/risc.c, the memory_write has been invoked twice to wrtie argc or args to memory (according to your comments). Why we need to maintain two copies of argc or args in virtual memory?

If we are running emulator on 64-bit machine, the virtual address would be 64-bit unless it is compile with -m32 option. Since we are RV32 emulator, so the address space should be < 4GB, from this point we cannot directly access 64-bit address from emulator. Thus, first call of memory write store the argc and argv to < 4GB address, and second call of memory write for RV32 ABI.

qwe661234 · 2023-08-08T02:58:53Z

If we are running emulator on 64-bit machine, the virtual address would be 64-bit unless it is compile with -m32 option. Since we are RV32 emulator, so the address space should be < 4GB, from this point we cannot directly access 64-bit address from emulator. Thus, first call of memory write store the argc and argv to < 4GB address, and second call of memory write for RV32 ABI.

Ok, but I still confuse. I guess these two copies are for different purposes. Could you explain the different purpose between store the argc and argv to < 4GB address and memory write for RV32 ABI?(or maybe leave some comments) Thx.

ChinYikMing · 2023-08-08T03:09:15Z

Ok, but I still confuse. I guess these two copies are for different purposes. Could you explain the different purpose between store the argc and argv to < 4GB address and memory write for RV32 ABI?(or maybe leave some comments) Thx.

For example, we want access the args[0], the instruction might look like this: lw a0, 4(sp). In mapping, emulator will call memory_read_w. From implementation of memory_read_w, it is base memory + "offset", this "offset" should within the memory size otherwise we get segmentation fault. Thus, value stored in sp+4 should be the offset to the args[0]. The first copy of argc and argv mainly for maintaining the offset and making sure it is < 4GB.

qwe661234 · 2023-08-08T03:25:15Z

For example, we want access the args[0], the instruction might look like this: lw a0, 4(sp). In mapping, emulator will call memory_read_w. From implementation of memory_read_w, it is base memory + "offset", this "offset" should within the memory size otherwise we get segmentation fault. Thus, value stored in sp+4 should be the offset to the args[0]. The first copy of argc and argv mainly for maintaining the offset and making sure it is < 4GB.

Does you mean the instruction lw a0, 4(sp) get the offset of memory address of args[0], and the data of args[0] store in this memory address(mem_base + offset)?

ChinYikMing · 2023-08-08T03:38:58Z

Does you mean the instruction lw a0, 4(sp) get the offset of memory address of args[0], and the data of args[0] store in this memory address(mem_base + offset)?

yes

ChinYikMing · 2023-08-08T15:33:08Z

Resolve the concerns raised by CodeQL.

I think I have solved it.

jserv · 2023-08-08T16:41:15Z

I defer to @qwe661234 for confirmation.

Makefile

jserv · 2023-08-09T12:13:13Z

Thank @ChinYikMing for contributing!

github-advanced-security bot found potential problems Aug 6, 2023

View reviewed changes

src/main.c Fixed Show fixed Hide fixed

src/main.c Fixed Show fixed Hide fixed

src/main.c Fixed Show fixed Hide fixed

src/main.c Fixed Show fixed Hide fixed

ChinYikMing requested a review from jserv August 6, 2023 23:06